feat(api): expose go api for third party apps by AndreyLevchenko · Pull Request #282 · aquasecurity/postee

AndreyLevchenko · 2022-02-07T11:44:42Z

Description of changes:

Exposed methods

implementation:
api.go
Technically more methods can be accessed but these ones are going to be documented
integration test:
api_integration_test.go

More structures were moved to `data` package

To keep model classes in single package. It's easy to find and helps to avoid cyclic dependencies.
Examples:

integrations.go
template.go
tenants.go

dbservice interface

It's required to support both bolt and postgres. Defined in dbservice.go
Existing bolt implementation is just moved to new package with minimal changes. Postgres implementation was created from scratch

Feature to store Postee yaml config in Postgres

implementation is here
cfgcachesource.go Config is fetched using postgresDb.TenantName as a key so single Postgres database can be used to store configs of unlimited amount of apps. (as long as they are using unique tenant names)

`CloneSettings()` method is added to output implementations.

Motivation is to provide clones instead of real objects for get/list API methods. This is to ensure that reference to Postee config object can not be used to modify app configuration directly (as it may lead to unpredictable results). Recommended way to update Postee config is using exposed API.

Logger package

All writes to log file are going through log package. Interface is defined here: logger.go
Also there is implementation based on zap logger: zaplogger.go

Misc msghandling change

format of webhook payload is changed from []byte to map[string]interface{}. This also requires change test data across many test classes

Questions:

Should Postgres implementatio be removed?
There is also a feature to store config yaml within Postgres db. It was specifically requested some time ago. Is it still required?

Co-authored-by: Andrey Levchenko levchenko.andrey@gmail.com
Co-authored-by: DmitriyLewen dmitriy.lewen@smartforce.io
Co-authored-by: DmitriyLewen 91113035+DmitriyLewen@users.noreply.github.com

* feat(api): add API draft more cleanup * added output api implementation * misc clean up * added routes and templates * switched API to synchronous mode * fixed typo * added simple integration test * fixed tests * added fix to set dbpath through API * added input callbacks & tests * fixed race condition * added misc fix to clean up input callbacks * fixed pointer reference error in loop * added default db path * fixed TODO in Jira outputs * feat(postgres): added support postgres * feat(postgresDb): add test connect psql, change cfg.yaml * feat(psql) changed configurate psql settings are now taken from env 'POSTGRES_URL' now,tests for configurate psql added * combined Webhooktable and WebhookExpiryDates * Refactor: code review notes are corrected * Refactor: code review notes are corrected * Refactor : added вудуеув temp dir in configureDb test * Docs(psql): added psql env info to readme * refactor: change parametrs for deleteRowsByIdAndTime * Refactor: code review notes are corrected * clean up of db connection config * exposed API to use postgres * added make target for golang-lint * feat:added copy of cfgFile in psql * test: changed tests for insert into psql * Refactor: code review notes are corrected * refactor: changed func load * misc api cleanup * test: added tests for api * test: added tests for configure psql * Refactor: code review notes are corrected * Refactor: code review notes are corrected * test: fixed api tests * test: fixed api tests * feat(logger): added default logger * feat: added custom logger * fix: fixed golangci-lint test errors * fix: fixed golangci-lint test errors * Refactor: code review notes are corrected * Refactor: code review notes are corrected * refactor: removed func debug * refactor: removed func debug * chore(deps): added zap dependency * fix: fixed load psql cfg with empty table * fix: fixed load psql cfg with empty table * test: added test for errors to getCfgCacheSource * test: added test for errors to getCfgCacheSource * fix: fixed lint error * fix: fixed test errors (#228) * reuse postgres db conncetion instance * adjust unit test for Postgres singleton and retrying functionality * reuse bolt open conn and adjust tests * remove postgres API and adjust unit tests * merge upstream main into postee-as-api-2 * revert output email dynamic port Co-authored-by: Andrey Levchenko <levchenko.andrey@gmail.com> Co-authored-by: DmitriyLewen <dmitriy.lewen@smartforce.io> Co-authored-by: DmitriyLewen <91113035+DmitriyLewen@users.noreply.github.com>

# Conflicts: # outputs/email.go

DmitriyLewen · 2022-02-08T10:28:28Z

router/router.go

+	synchronous            bool
+	inputCallBacks         map[string][]InputCallbackFunc
+	databaseCfgCacheSource *data.TenantSettings
+	bolt                   dbservice.DbProvider


DbProvider can be boltDb or PostgresDb. I think this field needs to be renamed.
Also we have this field here.
Сan we remove one of them?

Currently, we are not going to use Postgres integration within the Postee-as-lib, we are thinking of removing it completely from the code, so I named it bolt on purpose

@elad-da
To be honest I'm going to remove it. As duplicate references (ctx.bolt and dbservice.Db ) to same object may lead to confusion.

Please don't remove it, otherwise, the unit tests won't work as expected

DmitriyLewen · 2022-02-08T10:46:23Z

dbservice/boltdb/checker.go

+		c := b.Cursor()
+		size := 0
+		for k, v := c.First(); k != nil; k, v = c.Next() {
+			size += len(v)


we get the size in bytes, but readme says Mb.

DmitriyLewen · 2022-02-08T10:50:24Z

dbservice/boltdb/checker_test.go

+	}
+}
+
+func TestDbSizeLimnit(t *testing.T) {


misprint : Limit

DmitriyLewen · 2022-02-08T11:01:24Z

dbservice/boltdb/dbparam_test.go

@@ -0,0 +1 @@
+package boltdb


I think we don't need this file.

DmitriyLewen · 2022-02-08T11:23:38Z

dbservice/dbservice_test.go

-func TestStoreMessage(t *testing.T) {
-	var tests = []struct {
-		input *string
+func TestConfigurateBoltDbPathUsedEnv(t *testing.T) {


I think the name TestConfigurateBoltDb would be better

DmitriyLewen · 2022-02-08T11:23:55Z

dbservice/dbservice_test.go

-func TestInsertError(t *testing.T) {
-	var tests = []struct {
-		bucket string
+func TestConfiguratePostgresDbUrlAndTenantName(t *testing.T) {


I think the name TestConfiguratePostgresDb would be better

DmitriyLewen · 2022-02-08T12:01:35Z

msgservice/aggregatebytime_test.go

-		os.Remove(dbservice.DbPath)
-		dbservice.DbPath = dbPathReal
+		os.Remove(testDB.DbPath)
+		testDB.DbPath = dbPathReal


We don't need this. We return oldDb.

…t property

codecov · 2022-02-08T12:24:42Z

Codecov Report

Merging #282 (e8977c4) into main (1e41657) will decrease coverage by 1.92%.
The diff coverage is 82.25%.

❗ Current head e8977c4 differs from pull request most recent head 1a3bb16. Consider uploading reports for the commit 1a3bb16 to get more accurate results

@@            Coverage Diff             @@
##             main     #282      +/-   ##
==========================================
- Coverage   87.33%   85.40%   -1.93%     
==========================================
  Files          28       19       -9     
  Lines        1153     1165      +12     
==========================================
- Hits         1007      995      -12     
- Misses         88      100      +12     
- Partials       58       70      +12

Impacted Files	Coverage Δ
formatting/slackmrkdwnprovider.go	`78.26% <0.00%> (ø)`
regoservice/jsonformat.go	`35.29% <0.00%> (ø)`
routes/routes.go	`100.00% <ø> (ø)`
dbservice/dbservice.go	`50.00% <50.00%> (ø)`
msgservice/scheduler.go	`90.47% <75.00%> (ø)`
router/router.go	`82.32% <81.39%> (-4.48%)`	⬇️
msgservice/msghandling.go	`87.05% <81.81%> (-1.12%)`	⬇️
regoservice/eval.go	`79.59% <83.33%> (-0.41%)`	⬇️
router/parsecfg.go	`84.61% <83.33%> (-2.06%)`	⬇️
router/api.go	`90.32% <90.32%> (ø)`
... and 13 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb52428...1a3bb16. Read the comment docs.

DmitriyLewen · 2022-02-08T12:26:01Z

outputs/jira.go

 	return false
 }
+
+func cpyUnknowns(source map[string]string) map[string]string {


It is copy, if I understood correctly

it was attempt to make method name more short

DmitriyLewen · 2022-02-08T12:38:57Z

regoservice/eval.go

+	foundPaths := make([]string, 0)
+
+	for _, path := range commonRegoTemplates {
+		if _, err := os.Stat(commonRegoTemplates[0]); !os.IsNotExist(err) {


Why do we use [0]?
If i understood correctly, we need to check each path.

it can be one element or zero

DmitriyLewen · 2022-02-08T13:03:25Z

router/api.go

+func Send(b []byte) {
+	//Instance().Send(b)
+	Instance().handle(bytes.ReplaceAll(b, []byte{'`'}, []byte{'\''}))
+}


What does this function do?

it calls handler directly. no queue is involved

okay, no questions.

simar7

Description of changes:

Exposed methods

implementation: api.go Technically more methods can be accessed but these ones are going to be documented integration test: api_integration_test.go

More structures were moved to data package

To keep model classes in single package. It's easy to find and helps to avoid cyclic dependencies. Examples:

integrations.go

template.go

tenants.go

dbservice interface

It's required to support both bolt and postgres. Defined in dbservice.go Existing bolt implementation is just moved to new package with minimal changes. Postgres implementation was created from scratch

Feature to store Postee yaml config in Postgres

implementation is here cfgcachesource.go Config is fetched using postgresDb.TenantName as a key so single Postgres database can be used to store configs of unlimited amount of apps. (as long as they are using unique tenant names)

CloneSettings() method is added to output implementations.

Motivation is to provide clones instead of real objects for get/list API methods. This is to ensure that reference to Postee config object can not be used to modify app configuration directly (as it may lead to unpredictable results). Recommended way to update Postee config is using exposed API.

Logger package

All writes to log file are going through log package. Interface is defined here: logger.go Also there is implementation based on zap logger: zaplogger.go

Misc msghandling change

format of webhook payload is changed from []byte to map[string]interface{}. This also requires change test data across many test classes

Questions:

Should Postgres implementatio be removed?

There is also a feature to store config yaml within Postgres db. It was specifically requested some time ago. Is it still required?

Co-authored-by: Andrey Levchenko levchenko.andrey@gmail.com Co-authored-by: DmitriyLewen dmitriy.lewen@smartforce.io Co-authored-by: DmitriyLewen 91113035+DmitriyLewen@users.noreply.github.com

TBH - I think this PR is too big to be reviewed properly as it touches many different components. Can we break down into several manageable PRs as defined in the description above?

AndreyLevchenko · 2022-02-09T08:07:08Z

Hi
@simar7
Yes I can split it to several PRs like:

API itself
DB improvements (most probably I just drop it)
Logging

But I think it may take about a week as it means re-implementation of the features (almost from scratch). Also some new PRs could still be big enough to review. Let me know if it works for you.

simar7 · 2022-02-09T22:09:19Z

Hi @simar7 Yes I can split it to several PRs like:

API itself

DB improvements (most probably I just drop it)

Logging

But I think it may take about a week as it means re-implementation of the features (almost from scratch). Also some new PRs could still be big enough to review. Let me know if it works for you.

I'll touch base with @tomyweiss and @elad-da offline to get their thoughts and get back to you.

simar7 · 2022-02-10T18:04:05Z

Hi @simar7 Yes I can split it to several PRs like:

API itself

DB improvements (most probably I just drop it)

Logging

But I think it may take about a week as it means re-implementation of the features (almost from scratch). Also some new PRs could still be big enough to review. Let me know if it works for you.

I'll touch base with @tomyweiss and @elad-da offline to get their thoughts and get back to you.

Update: We'll discuss it in a meeting next week - so please hold off until then.

# Conflicts: # go.mod # go.sum # router/initoutputs_test.go

* Embed rego-templates instead of reading directly from the file system (postee as lib usage) * Embedding the rego templates and load it to the rego eval in case the files not found on the filesystem * expose API - get embedded templates files * revert rego templates folder name * fix rego-templates import path * Fix typos and generalize functions * embedded template unit tests

aquasecurity#282) Bumps [github.com/aquasecurity/defsec](https://github.com/aquasecurity/defsec) from 0.68.6 to 0.68.9. - [Release notes](https://github.com/aquasecurity/defsec/releases) - [Commits](aquasecurity/defsec@v0.68.6...v0.68.9) --- updated-dependencies: - dependency-name: github.com/aquasecurity/defsec dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

AndreyLevchenko changed the title ~~Main & postee API merge (#263)~~ feat(api): expose go api for third party apps Feb 7, 2022

AndreyLevchenko added 4 commits February 7, 2022 18:17

Merge branch 'main' of github.com:aquasecurity/postee into dev

098b20f

# Conflicts: # outputs/email.go

fix tests and linter errors

8d548cb

add missed test

2ac2f7d

remove PostgreSQL references from README.me

29e2528

AndreyLevchenko requested review from simar7 and tomyweiss February 8, 2022 08:50

DmitriyLewen reviewed Feb 8, 2022

View reviewed changes

dbservice/boltdb/checker_test.go Outdated

}

}

func TestDbSizeLimnit(t *testing.T) {

Copy link
Copy Markdown

Collaborator

DmitriyLewen Feb 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

misprint : Limit

DmitriyLewen reviewed Feb 8, 2022

View reviewed changes

fix tests to avoid using default path '/server/..', remove router.bol…

8768a4d

…t property

DmitriyLewen reviewed Feb 8, 2022

View reviewed changes

rename test methods, clean up not useful code

2edb888

DmitriyLewen reviewed Feb 8, 2022

View reviewed changes

AndreyLevchenko added 3 commits February 8, 2022 19:19

fix db size limit measurement unit, fix test

f0f5488

fix typo

75c2c11

move existing method to logger package

e8977c4

simar7 requested changes Feb 9, 2022

View reviewed changes

AndreyLevchenko mentioned this pull request Feb 15, 2022

Postee as api 2 #163

Closed

Merge branch 'main' of github.com:aquasecurity/postee into dev

bc4dfe1

# Conflicts: # go.mod # go.sum # router/initoutputs_test.go

AndreyLevchenko and others added 3 commits February 16, 2022 15:42

adopted latest changes

b8cb265

removed go.mod and go.sum from spell checking

bfbacda

Conversation

AndreyLevchenko commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes:

Exposed methods

More structures were moved to data package

dbservice interface

Feature to store Postee yaml config in Postgres

CloneSettings() method is added to output implementations.

Logger package

Misc msghandling change

Questions:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simar7 left a comment

Choose a reason for hiding this comment

Description of changes:

Exposed methods

More structures were moved to data package

dbservice interface

Feature to store Postee yaml config in Postgres

CloneSettings() method is added to output implementations.

Logger package

Misc msghandling change

Questions:

Uh oh!

AndreyLevchenko commented Feb 9, 2022

Uh oh!

simar7 commented Feb 9, 2022

Uh oh!

simar7 commented Feb 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

AndreyLevchenko commented Feb 7, 2022 •

edited

Loading

More structures were moved to `data` package

`CloneSettings()` method is added to output implementations.

codecov bot commented Feb 8, 2022 •

edited

Loading

More structures were moved to `data` package

`CloneSettings()` method is added to output implementations.